Skip to content

Conversation

@giacomosirri
Copy link
Contributor

This Pull Request implements the feature requested in the issue #3021

@giacomosirri giacomosirri requested review from a team and remyleone as code owners April 7, 2025 08:17
@github-actions github-actions bot added the iam IAM issues, bugs and feature requests label Apr 7, 2025
@remyleone remyleone changed the title feature #3021: (IAM) Creation of user as Member feat(iam): creation of user as Member Apr 7, 2025
@crlptl
Copy link

crlptl commented Apr 7, 2025

Hello @giacomosirri and thank you for this PR.
The need of TF implementation for IAM members is real and we need to work on it ASAP.
We will not accept the contribution as it stands because it would have implications on member feature's future that we see differently - for example we may implement a dedicated resource scaleway_iam_member. Someone from the Scaleway team will work on this resource in the next weeks.

The guest type will disappear in the next months, and the fields you mentioned that cannot be updated will be updatable in very soon too.

However, I would like to thank you for your contribution. Can you give me your Organization-id (here or on Slack Scaleway community - Cyril Scw) so that I can put a voucher on your Organization? Thanks again

@giacomosirri
Copy link
Contributor Author

Hello @crlptl,
Thank you very much for the feedback and the kind voucher proposal. I understand what you mean, it is a complex matter that probably requires more careful considerations, so I look forward to see these features fully implemented.

Anyway, the organization id is: daf36079-e52c-416c-9535-d06742e48acc.

Btw, I also tried to access the slack community from the link in the footer of the website, but it appears to be broken:
image

Thank you again.

@remyleone remyleone closed this Apr 7, 2025
@Gnoale
Copy link
Contributor

Gnoale commented Apr 30, 2025

Hello @giacomosirri, after careful consideration it happens that your approach fits well the current API state and is future proof.
I re open your PR and will make a few suggestions if you are still willing to submit it.
Thank you.

@Gnoale Gnoale reopened this Apr 30, 2025
@Gnoale Gnoale force-pushed the member-user-creation branch from cbafdfd to 9fcac44 Compare April 30, 2025 15:17
Copy link
Contributor

@Gnoale Gnoale left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution, I made a few suggestion, feel free to tell me if you don't have time to work on this I would then make the changes myself

@giacomosirri
Copy link
Contributor Author

Hello @Gnoale, thank you for the update, that's good to hear!
I will take the time to work on it and update the code based on your suggestions. I might be able to submit it within this week, at the latest next week.

@giacomosirri giacomosirri force-pushed the member-user-creation branch 2 times, most recently from 3282040 to 344f1e8 Compare May 15, 2025 10:55
@giacomosirri
Copy link
Contributor Author

@Gnoale @remyleone hello,
I'm pinging you here just to let you know that I have implemented the requested changes.
The acceptance tests were all ok and as far as I can tell now everything works pretty much as I would expect. However, if you notice something wrong, please let me know so that I can fix it!
Thank you

Copy link
Contributor

@Gnoale Gnoale left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @giacomosirri thanks for you work, I made another review pass, it is almost good I think

I suggest we remove the ForceNew flag from the "email" field and return an error in case of a guest email update attempt.
Since guest will be deprecated

We could add a hint about this behavior in the documentation too

What do you think @remyleone @crlptl

@giacomosirri giacomosirri force-pushed the member-user-creation branch from 344f1e8 to cb2d847 Compare May 19, 2025 15:28
@giacomosirri
Copy link
Contributor Author

@Gnoale @remyleone hello,
I have updated again the implementation so that it now fully matches the API. I have also made minor changes to the documentation, now it should be a bit more clear. Let me know if it's ok for merging.
Thank you

Copy link
Contributor

@Gnoale Gnoale left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM !

@Gnoale Gnoale added this pull request to the merge queue May 20, 2025
Merged via the queue into scaleway:master with commit e782e5c May 20, 2025
46 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

iam IAM issues, bugs and feature requests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants